-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pages Editor: add "Delete Task from Page" functionality #7075
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I'm able to edit a step by adding or removing a task, and if I delete a lone task, its step is also deleted. When I delete a task that's linked to a previous step, that previous step is automatically modified as expected 👍
|
||
// Cleanup, then commit. | ||
const cleanedTasksAndSteps = cleanupTasksAndSteps(newTasks, newSteps); | ||
update(cleanedTasksAndSteps); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like how the code comments in this function are easy to read through 👍
export default function cleanupTasksAndSteps(tasks = {}, steps = []) { | ||
const newTasks = structuredClone(tasks); // Copy tasks | ||
let newSteps = steps.slice(); // Copy steps | ||
let newSteps = structuredClone(steps); // Copy steps. This is a deep copy, compared to steps.slice() | ||
|
||
// Remove steps without tasks. | ||
newSteps = newSteps.filter(step => step?.[1]?.taskKeys?.length > 0); | ||
|
||
// Remove tasks not associated with any step. | ||
Object.keys(newTasks).forEach(taskKey => { | ||
let existsInAnyStep = false; | ||
newSteps.forEach(step => { | ||
existsInAnyStep = existsInAnyStep || !!step?.[1]?.taskKeys?.includes(taskKey); | ||
}); | ||
if (!existsInAnyStep) delete newTasks[taskKey]; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just linking this FEM Issue about orphaned tasks for documentation. Your implementation here means any project using the Pages Editor solves the bug 🎉 Hopefully all FEM projects soon 🤞
ae9d5d1
to
f3db498
Compare
…sksAndSteps: fix order of actions.
f3db498
to
22dcb15
Compare
Thanks Delilah! 👍 |
PR Overview
Part of: Pages Editor MVP project and FEM Lab super-project
Follows #7065
Staging branch URL: https://pr-7075.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging
This PR implements the "Delete Task" functionality, which deletes Tasks from an existing Page. Prior to this PR, you only had the "Delete Page/Step" functionality.
New Behaviour:
cleanupTasksAndSteps()
now properly removes Steps/Pages that have 0 Tasks, and removes Tasks that aren't associated with any Page/Step.Testing Steps
Status
Ready for review!